- 
                Notifications
    You must be signed in to change notification settings 
- Fork 3.9k
Introduce and extend Config pattern (III/2) #2368
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
27da3e7    to
    c28a0b9      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks for going through the comments from the prevision revision! Just a few more minor comments here and then I am happy to sign off
        
          
                src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                src/test/java/redis/clients/jedis/tests/ShardedJedisPoolTest.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                src/test/java/redis/clients/jedis/tests/ShardedJedisPoolWithCompleteCredentialsTest.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
      3b6d054    to
    22f3f0f      
    Compare
  
    22f3f0f    to
    c542793      
    Compare
  
            
          
                src/test/java/redis/clients/jedis/tests/ShardedJedisPoolWithCompleteCredentialsTest.java
          
            Show resolved
            Hide resolved
        
      | 
 These tests used to work because underlying connections were not always created given the combination of parameters. All the Jedis classes were suffering from this situation. The underlying connection of object representing a connection should either always or NEVER be created from constructor. In this PR I implemented the "always" approach. I was able to keep the tests with a valid host address in (3) and (4). But I could not make (1) and (2) to work. Even though I  This is more logical that  IMHO, we shouldn't care about impractical tests. Still, I can  | 
Resolves #1780
Resolves #1824
Resolves #2234
Closes #1787
Closes #2237
Closes #2342
Closes #2349
Closes #2352
Closes #2353
Closes #2363